-
Notifications
You must be signed in to change notification settings - Fork 41
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Init/Finalize Clarification #37
Init/Finalize Clarification #37
Conversation
Remove passage from API notes that contradicts the following semantic from the API description of shmem_init: "At the end of the OpenSHMEM program which it initialized, the call to shmem_init must be matched with a call to shmem_finalize." Signed-off-by: James Dinan <james.dinan@intel.com>
Signed-off-by: James Dinan <james.dinan@intel.com>
Remove the following from finalize notes:
|
Update mention of shmem_finalize in Annex A |
content/backmatter.tex
Outdated
@@ -440,6 +440,10 @@ \section{Version 1.5} | |||
|
|||
\begin{itemize} | |||
% | |||
\item Clarified that \FUNC{shmem\_init} must be matched with a call to | |||
\FUNC{shmem\_finalize}. | |||
\\See Section \ref{subsec:shmem_init}. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also ref shmem_finalize
Signed-off-by: James Dinan <james.dinan@intel.com>
Signed-off-by: James Dinan <james.dinan@intel.com>
Signed-off-by: James Dinan <james.dinan@intel.com>
I'm bit confused with this change...Just to be sure, in this change we are not removing implicit finalize from the spec? |
This change removes implicit finalize in cases where the library was initialized with a call to |
At present, we have information about "implicit finalize" at two places - at shmem_init and shmem_finalize explanations. With this change, we seem to remove this information at both these places. At least, to me it looks like we have removed implicit finalize completely from the spec. |
Do you have an objection to the semantic that I mentioned in my previous comment, or are you requesting that we update the deprecated interfaces section to include implicit finalize as part of this proposal? Or something else? I don't mind updating the deprecated interfaces documentation, including the |
We are removing implicit finalize usage along with shmem_init in this PR. I don't want this semantics change without fixing the backward incompatibility issues.
At present, we require every shmem_init to be matched with shmem_finalize. But, we don't specifically say whether the finalize comes from explicit or implicit call. But this PR tends to make the explicit shmem_finalize usage a required model. I like the intention behind this PR and this clarification is also important. So let us add new routines: shmem_finalized and shmem_initialized like MPI_initialized and MPI_finalized along with this change and completely remove implicit finalize from the specification. |
FWIU, as per the current specification the following is the desired and allowed usage model.
As you said, the current spec is inadequate with the following unanswered question:
This PR tries to clarify the above question, by removing details about implicit finalize from all places w.r.t to shmem_init. To enhance this clarification, as you suggested we should update the deprecated interfaces documentation, by including the start_pes section and also explicitly telling implicit finalize is valid only with start_pes. |
I take back my previous request, I don't have any use for implicit finalize now ;)
|
Reference counting requires the library to support re-initialization, because of applications that may use more than one SHMEM library:
I suggest that we split these up -- use this ticket to clarify the current semantics as you described above and start a second ticket for reference counting. Would you be ok with that approach? |
Signed-off-by: James Dinan <james.dinan@intel.com>
@naveen-rn Could you please review the updated draft? |
Looks fine to me. |
content/execution_model.tex
Outdated
resources without terminating the program. Calling any \openshmem routine after | ||
\FUNC{shmem\_finalize} leads to undefined behavior. | ||
program concludes its use of the \openshmem library when all \acp{PE} call | ||
\FUNC{shmem\_finalize} or when any \ac{PE} calls \FUNC{shmem\_global\_exit}. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From @shamisp - Do we want to preserve the bit about completing all pending communication and releasing all resources in finalize?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
Signed-off-by: James Dinan <james.dinan@intel.com>
Special ballot changes: 53843d4 |
content/execution_model.tex
Outdated
program finishes execution by returning from the main routine or when any PE | ||
calls \FUNC{shmem\_global\_exit}. When returning from main, \openshmem must | ||
program concludes its use of the \openshmem library when all \acp{PE} call | ||
\FUNC{shmem\_finalize} or when any \ac{PE} calls \FUNC{shmem\_global\_exit}. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Split into two sentences: "In addition, the program can be aborted when any PE calls shmem_global_exit.
"
content/execution_model.tex
Outdated
calls \FUNC{shmem\_global\_exit}. When returning from main, \openshmem must | ||
program concludes its use of the \openshmem library when all \acp{PE} call | ||
\FUNC{shmem\_finalize} or when any \ac{PE} calls \FUNC{shmem\_global\_exit}. | ||
During finalization, \openshmem must |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggested
- "finalization" -> "shmem_finalize"
- "openshmem" -> "the openshmem library"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in c1dfa16, please take a look!
Signed-off-by: James Dinan <james.dinan@intel.com>
Return values and error checking
Return values and error checking
Clarify that
shmem_init
must be matched with a call toshmem_finalize
.